Allow STM32 DCMIPP to work with UVC sample #94562
Conversation
|
avolmat-st
force-pushed
the
pr-dcmipp-caps-uvc-range
branch
from
fcd6732 to
371ef0e
Compare
|
Simply rebased for the time being, removing the DCMIPP Planar commit which are not already merged. |
The UVC class was deciding itself which formats were sent to the host. Remove this logic out of the UVC class and introduce uvc_add_format() to give the application the freedom of which format to list. Signed-off-by: Josuah Demangeon <[email protected]>
The UVC class now lets the application select the format list sent to the host. Leverage this in the sample to filter out any format that is not expected to work (buffer too big, rarely supported formats). Signed-off-by: Josuah Demangeon <[email protected]>
Add USB UVC device's new uvc_add_format() function to the release note, and document the semantic changes of UVC APIs in the migration guide. Signed-off-by: Josuah Demangeon <[email protected]>
avolmat-st
force-pushed
the
pr-dcmipp-caps-uvc-range
branch
from
371ef0e to
99218f4
Compare
zephyrbot
added
Release Notes
zephyrbot
requested review from
FRASTM,
GeorgeCGV,
cfriedt,
djiatsaf-st,
erwango,
etienne-lms,
gautierg-st,
jfischer-no,
jhedberg,
loicpoulain,
marwaiehm-st,
mathieuchopstm and
tmon-nordic
|
Adding |
| static const struct video_format_cap stm32_dcmipp_main_fmts[] = { | ||
| DCMIPP_VIDEO_FORMAT_CAP(RGB565, 8), | ||
| DCMIPP_VIDEO_FORMAT_CAP(YUYV, 8), | ||
| DCMIPP_VIDEO_FORMAT_CAP(YVYU, 8), | ||
| DCMIPP_VIDEO_FORMAT_CAP(GREY, 16), | ||
| DCMIPP_VIDEO_FORMAT_CAP(RGB24, 16), | ||
| DCMIPP_VIDEO_FORMAT_CAP(BGR24, 16), | ||
| DCMIPP_VIDEO_FORMAT_CAP(ARGB32, 4), | ||
| DCMIPP_VIDEO_FORMAT_CAP(ABGR32, 4), | ||
| DCMIPP_VIDEO_FORMAT_CAP(RGBA32, 4), | ||
| DCMIPP_VIDEO_FORMAT_CAP(BGRA32, 4), | ||
| DCMIPP_VIDEO_FORMAT_CAP(NV12, 16), | ||
| DCMIPP_VIDEO_FORMAT_CAP(NV21, 16), | ||
| DCMIPP_VIDEO_FORMAT_CAP(NV16, 16), | ||
| DCMIPP_VIDEO_FORMAT_CAP(NV61, 16), | ||
| DCMIPP_VIDEO_FORMAT_CAP(YUV420, 16), | ||
| DCMIPP_VIDEO_FORMAT_CAP(YVU420, 16), | ||
| {0}, | ||
| }; |
There was a problem hiding this comment.
The stm32_dcmipp_aux_fmts is a subset of this. Can we declare stm32_dcmipp_aux_fmts first and include it inside this to save some memory, something like:
static const struct video_format_cap *stm32_dcmipp_main_fmts[] = {
&stm32_dcmipp_aux_fmts[0], /* RGB565 */
...
&stm32_dcmipp_aux_fmts[9], /* BGRA32 */
DCMIPP_VIDEO_FORMAT_CAP(NV21, 16),
...
};
There was a problem hiding this comment.
This seems to me quite error prone. We would be mixing up static const struct video_format_cap table of structure for the aux and table of pointers for the main. Moreover if for some reason we add / delete on aux then we need to ensure it is ok as well on the main. So I feel like it is better to stick to this version which might indeed consume a bit more code space.
There was a problem hiding this comment.
This can all be optimized as soon as there is a video_enum_pixfmt() + video_enum_frmsize() API? :D
| * Give here the multiple in number of pixels, which depends on the format chosen | ||
| */ | ||
| #define DCMIPP_CEIL_DIV_ROUND_UP_MUL(val, div, mul) \ | ||
| ((((val) + (div) - 1) / (div) + (mul) - 1) / (mul) * (mul)) |
There was a problem hiding this comment.
nit: should we add a new line after each #define for better visibility ?
include/zephyr/drivers/video.h
Outdated
| * @param dev Video device to query. | ||
| * @param fmt Video format structure pointer |
There was a problem hiding this comment.
@param dev Pointer to the video device struct to set format ? Should document also @return or @retval ?
include/zephyr/drivers/video.h
Outdated
| * compose rectangle before being able to apply the frame format (which must have the | ||
| * same width / height and the compose rectangle width / height. |
There was a problem hiding this comment.
which must have the same width / height **with / as** the compose rectangle width / height ... ?
Currently the DCMIPP driver rely on a Kconfig in order to
select the right sensor resolution / format to pick.
This also makes the exposure of caps easier since it can
be exposed as:
DUMP pipe: same caps as mentioned in Kconfig
MAIN pipe: any format supported on this pipe and resolution
starting at sensor selected resolution down to
64 times smaller (which is the maximum of the
downscale)
AUX pipe: same as MAIN except without the semi-planar and
planar formats
Signed-off-by: Alain Volmat <[email protected]>
Some devices allow for downscale / upscale via the set_selection compose API. When using it, it is necessary to perform a set_selection of the compose target prior to setting the format. In order to allow non-compose aware application to benefit from it, introduce a helper which take care of setting the compose prior to setting the format. Signed-off-by: Alain Volmat <[email protected]>
Simplify the code by using the video_set_compose_format helper. Signed-off-by: Alain Volmat <[email protected]>
Honor the CONFIG_VIDEO_BUFFER_POOL_ALIGN config by using the video_buffer_aligned_alloc function instead of video_buffer_alloc in order to provide properly aligned buffers to drivers. Signed-off-by: Alain Volmat <[email protected]>
Use the helper video_set_compose_format in order to allow controlling the compose. Signed-off-by: Alain Volmat <[email protected]>
Select from commonly used resolution when the video device advertise capabilities using range. Signed-off-by: Alain Volmat <[email protected]>
Add board specific conf files for the stm32n6570_dk Signed-off-by: Alain Volmat <[email protected]>
avolmat-st
force-pushed
the
pr-dcmipp-caps-uvc-range
branch
from
99218f4 to
fb51771
Compare
|
7 participants
This PR contains various area commits.
I keep this in draft since this is still work in progress and I'd like to open the discussion about those topics.
I added the
video_set_compose_formathelper (probably not the best name) in order to add automatically the call to set_selection. However we might want, in the same helper or not to go even further of that and, if the device support it use crop to achieve the format requested, there could also be an option to for example keep aspect-ratio, which do the heavy lifting of checking the native resolution and perform the crop / compose in order to get a frame with the requested resolution while keeping aspect-ratio (if it has been asked to).I used this new helper on the UVC sample as well, making it easy to select the right resolution.
When a device is advertising a range of width/height, the current UVC sample only expose the max and min resolutions (assuming they can be achieved from a memory point of view).
There would be various way to achieve computing of intermediate resolutions, computing the aspect ratio as well, depending on the step value advertised by the device the amount of intermediate resolution requested (or should I say limited).
In a trial to keep it simple, I put instead a list of commonly used resolution and have the sample app simply check if this can be achieved or not.
This PR depends on 2 other PRs, PR #93192 and PR #94081 so I merged them at the beginning so only the last 6 commits are relevant in this serie.